-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ANCHOR-916] Put Horizon behind interface #1624
base: epic/anchor-937-stellar-rpc-support
Are you sure you want to change the base?
[ANCHOR-916] Put Horizon behind interface #1624
Conversation
f5dca97
to
463ae5b
Compare
* @return The transaction response. | ||
* @throws NetworkException | ||
*/ | ||
TransactionResponse submitTransaction(Transaction transaction) throws NetworkException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransactionResponse
is Horizon's submitTransaction
response. RPC's sendTransaction
returns a SendTransactionResponse
which only contains the transaction hash and its status. We will need to call getTransaction
to fetch the transaction afterward. The schemas are not the same. Should we create a new type to encapsulate the transaction submission responses?
import org.stellar.sdk.responses.TransactionResponse; | ||
import org.stellar.sdk.responses.operations.OperationResponse; | ||
|
||
public interface LedgerApi { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SEP-45 uses transaction simulation which is only available in RPC. How do we plan to expose RPC-specific methods in the interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In SEP-45, it should work directly with the implementation class instead of LedgerApi.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And Horizon option (horizon_url
) will cause conflict and fail to start during validation of sep45.enabled
is true
.
* @param stellarTxnId The Stellar transaction ID. | ||
* @return The operations for the transaction. | ||
*/ | ||
List<OperationResponse> getStellarTxnOperations(String stellarTxnId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OperationResponse
is Horizon's getOperation
response. If we use RPC, we must parse the operations from the transaction envelope. Should we create a new type to encapsulate the operation response?
private AccountEntry fetchAccountEntry(String account) throws IOException { | ||
// TODO: Implement this method | ||
return null; | ||
// GetLedgerEntriesResponse response; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to put the Horizon behind the interface. Will add tests after the StellarRpc
implementation in a different PR.
} | ||
|
||
/** Increments sequence number in this object by one. */ | ||
public void incrementSequenceNumber() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required by the TransactionBuilderAccount
interface.
Also, for the branch name. I think we typically call long-lived feature branches |
Description
LedgerApi
interfaceContext
Testing
./gradlew test
Documentation
N/A
Known limitations
N/A